Fix two observer-verification gaps - #306

ensureObserver loads the observer record, awaits verifier RPCs (and possibly
the configuration modal, which parks on user input indefinitely), then persists
the record. Input gates don't cover those awaits, so two concurrent opens for
one profile raced: two first opens each minted their own observerId,
registering both with the gatekeepers while the last-written record forgot the
other id existed, and a later open's final put could overwrite state a
concurrent open had just written.

A per-profile promise chain now serializes the whole body, following the
existing #preparingChatMessages pattern. blockConcurrencyWhile is not usable
here: it would freeze the entire DO for an unbounded modal wait. Distinct
profiles stay concurrent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Exercises the authorizeCollaborator gate end-to-end through the real
ExternalMessageGateway entrypoint: an unverified build collaborator is refused
until they open the workspace (which verifies them), and a "use" collaborator
is denied by role before verification ever runs.

The fixture worker grows a control surface for submitting external messages
(a service binding to the Workshop's gateway entrypoint, plus an Overseer
namespace binding used only to derive the workspace id behind a gadgetKey),
and its sessions become real: readThing()/doThing() drive observations and
actions through the same ApprovalQueue funnel a shipping gatekeeper uses.

Also hardens the harness against local-dev leakage: worker configs declare an
empty required-secrets list so a developer's .dev.vars (say CF_AI_GATEWAY_*)
can't change suite behavior -- these tests depend on no test user having an
AI model.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…tion.

The suite's final assertion (Bob passes the gate after opening in a browser)
would pass identically if the external path only checked the persisted
observer record. Flip the fixture's verify outcome for Bob after his open and
assert the denial carries the fixture's own refusal reason -- which nothing
persisted in the Workshop contains, so it can only come from a live
addObserver round trip. Pins the revocation catch that is the point of the
external-path verification.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The per-profile chain links queued ensureObserver calls through a promise
resolved by release() in a finally, so a failing verification can never
poison or deadlock the queue -- but no test pinned that. Queue a second
same-profile open behind one whose configuration modal throws, and assert it
completes as an ordinary first open with its own choices persisted.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

authorizeCollaborator captured the caller's role, then awaited
ensureObserver -- which parks across real await windows (verifier RPCs,
even the configuration modal) -- and returned the stale capture. A
collaborator removed in that window was admitted at their stale role if
the park resolved before the revocation restart landed -- and worse,
ensureObserver's step-6 record persist blindly resurrected the observer
record the removal's teardown had just deleted (record and account
choices were loaded pre-park), leaving a removed user with coverage that
a later re-grant would trust without re-verification. An external prompt
could thereby be durably committed and its agent turn resumed after the
restart.

ensureObserver now takes an optional commit gate, run synchronously at
each success exit -- immediately before the step-6 put, or at the
nothing-to-verify early return -- inside the per-profile verification
lock, so a denial throws into the existing rollback and nothing lands
between a passing gate and the persist. authorizeCollaborator's gate
re-checks the live effective role, and the returned capability is
re-derived afterward: a mid-park removal is denied at commit time, and a
mid-park downgrade caps the returned role at the live one (which
verification at the wider pre-park scope covers); an upgrade takes
effect at the next open, which verifies at the wider scope.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…commit.

receiveExternalMessage checked authorizeCollaborator, then crossed real
awaits (owner registration, the caller's context RPC, message
preparation) before sendChatMessage/newChat committed the prompt and
startAgent ran over the unfiltered chat tail. A sharing change landing
in that window severed the caller's role (and tore down their observer
record) -- or a new connection widened the scope they were never
verified against -- yet nothing re-checked, and the reply left the
Workshop.

The response-target registration now carries an assertStillAuthorized
closure -- wrapping the new synchronous assertCollaboratorStillVerified,
which mirrors ensureObserver's success invariant (effective role, plus
full observer-record coverage of the live-recomputed scope, so a
connection added mid-flight fails closed) -- run by newChat as the first
statement of the transaction that writes the prompt, and by
sendChatMessage just before materializeChatChanges: its first write,
which cannot move inside the transaction (non-transactional side
effects), so the check is hoisted with no awaits between it and the
transaction. A stale caller therefore commits nothing: no message (not
even a materialized "changes" one), no chat, no response target, and no
agent turn.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

A collaborator whose provider-side access had been revoked kept a persisted
observer record listing the gatekeeper, so the commit-time re-check
(assertCollaboratorStillVerified) kept treating them as verified: their
still-live sessions' external-message writes were admitted even after a live
addObserver had refused them.

The failed gatekeeper is now dropped from that record synchronously with the
failure determination, and the terminal catch de-registers invalidated
gatekeepers alongside newly-added ones.

Fail-closed by design: an outage or expired credential scrubs the same way,
blocking that collaborator's external-message writes until they re-open
successfully.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The scrub-on-failure and the per-profile serialization compose: a concurrent
open's success must not resurrect coverage a failed live check just scrubbed.
Asserts the interaction now that both exist.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

In ensureObserver's per-gatekeeper verify, the getVerifier await sat outside
the try whose catch scrubs the persisted coverage, so a rejection there --
deterministic on the User DO's vendor-mismatch throw, or any cross-worker
transport failure -- denied the open but left the persisted accountChoices
entry intact: assertCollaboratorStillVerified kept treating the collaborator
as verified for that gatekeeper on their older live sessions, the exact
stale-coverage hole the scrub exists to close.

The await moves inside the try, so every failure of the verify goes through
fail(): coverage is scrubbed for the failed gatekeeper only, the failure gets
the re-prompt/#describeObserverFailures treatment instead of leaking the raw
RPC error, and the callbacks no longer reject -- so Promise.all can't reject
mid-flight and the terminal catch's newlyAdded/invalidated rollback snapshot
can no longer miss registrations that complete after a sibling's rejection.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

ensureObserver verified only the collaborator's in-scope gatekeepers but
kept (and re-persisted) account choices for everything else, so a "use"
collaborator opening while a connection was unbound from every gadget
re-verified nothing against it yet kept their stale entry. Rebinding the
connection keeps the same gatekeeper id (only gadget binding edges
change), so a gatekeeper unbound and rebound between an entry gate and
its chat commit would leave assertCollaboratorStillVerified trusting an
entry the collaborator's most recent open never verified.

Prune out-of-scope entries from the persisted record at every open --
including an empty-scope open, which is exactly the everything-unbound
case -- restoring the invariant "entry present => verified at this
collaborator's most recent open". The gatekeeper-side registration is
kept (forward exclusion via byObserverId), as is the record itself even
when its accountChoices empties.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…led re-verification.

ensureObserver's terminal catch deregistered newlyAdded ∪ invalidated
gatekeeper-side unconditionally. For a re-verification failure the removed
registration is what preserves forward exclusion: ObserverTracker loses the
id, prepareObservation stops naming the observer in excludeObservers, and
their still-live session (a failed re-verification never restarts sessions)
sees later excluded observations. Commit-time re-checks were never at risk --
coverage is scrubbed synchronously in fail() -- so keeping the registration
is fail-closed (it can only add exclusion names) and self-heals (the next
successful open's addObserver overwrites the verifier).

The rollback now runs only for a first-ever verification (!record, the same
discriminator as #pendingObserverIds): that collaborator was never admitted,
has no live session, and the minted id would otherwise linger unresolvable
inside the gatekeepers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

authorizeObservation awaited #enforceExcludeObservers -- whose teardown loop
awaits a cross-worker removeObserver fan-out per lost observer -- between the
exclusion check and the action record, so a re-grant landing in that window
admitted an observation naming a collaborator who was authorized again by the
time it was recorded. The method now takes the sharing manager once up top and
runs the exclusion decision, the prohibitAllSharing latch, and the action
record in one synchronous block (the house rule -- cf. addCollaborator):
enforcement splits into a synchronous #decideExcludeObservers plus a
#tearDownExcludedObservers awaited after the writes (still awaited, not
waitUntil: ApprovalQueueImpl returns this promise to gatekeeper workers, so an
admitted observation implies the teardown ran before data flows).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The DO abort that ends a removed collaborator's live sessions runs only
after removeCollaborator/revokeShareLink await tearDownLostObservers (a
serial removeObserver fan-out) and refreshAffectedCollaboratorListings
(chunked cross-DO round trips) -- a window that scales with collaborator
and gatekeeper count, not the ~100ms the comments claimed. Inside it,
the exclusion gate reads the removed user as already gone: their record
is deleted at the sever, so #decideExcludeObservers treats their
observerId as unknown and admits an observation naming exactly them --
all while their session still watches the fan-out, violating the
documented gatekeeper contract.

tearDownLostObservers now sets an in-memory #revocationRestartPending
flag synchronously with the sever (same predicate as the restart,
downgrades included), and the gate fails closed while it is set. The
flag is never cleared: the abort destroys it with the DO, and if the
restart were somehow lost, staying blocked is the safe direction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The excludeObservers id list crosses the RPC boundary from gatekeeper
code, so nothing guarantees uniqueness (the in-tree gatekeepers happen
to send unique ids), and #tearDownExcludedObservers deleted by profileId
from a snapshot staled by each iteration's awaited removeObserver
fan-out. A duplicate id -- or a second lost observer later in the list
-- whose profile was re-granted and re-verified inside that await had
its *replacement* record (new observerId) deleted, after which every
exclusion naming the new id silently no-oped, fail-open, until the
user's next open.

Two narrow fixes: #decideExcludeObservers dedupes the externally
supplied ids, and the teardown re-reads per iteration and deletes only
on an observerId match; the snapshotted id is still de-registered
unconditionally (removeObserver is idempotent and the id is dead either
way).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

open() ran ensureObserver with no commit gate and never re-read the role
after verification, unlike authorizeCollaborator: a removal landing while
the verification was parked (verifier RPCs, the configuration modal) let
step 6's put resurrect the observer record the removal's teardown had just
deleted, and the open handed out a capability selected by the stale
pre-park role -- a mid-park downgrade to "use" still received the full
OverseerClientInterface until the revocation restart landed.

open() now passes the same live-role commit gate authorizeCollaborator
uses (redeemShareKey writes a live edge before the role read here, so the
live-graph re-check is the whole story), re-derives the role after
verification, and caps the capability at the live role. Tests drive the
production open() entry point against the real DO for both the removal
and the downgrade.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…down mid-verification.

The keep-registrations rule (see the catch's comment) rests on byObserverId
continuing to resolve the kept id. When a removal's teardown deletes the
record while a re-verification is parked, the registrations that call just
re-asserted reference an id no record resolves: the observer survives
registered gatekeeper-side but unresolvable -- a retained verifier for a
removed user, an id even the revocation restart never cleans up, and a
stale verifier whose rejection can block reads.

The catch now re-reads the record: when the id this call anchored on no
longer matches, the full in-scope registration set is removed. Issued after
step 5 settled, so it cannot lose to an in-flight addObserver.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

… test.

addCollaborator resolves null when the share fails, and the stranger-denial
case above asserts the identical /do not have access/ message, so a silent
null made this case vacuous.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

`#revocationRestartPending` was consulted only inside `#decideExcludeObservers`,
which runs only when the observation names `excludeObservers`. But once the
teardown's removeObserver fan-out completes, gatekeepers no longer know the
removed user's observer id, so their subsequent observations arrive with no
exclusion naming them and were admitted -- while the revoked session stays live
until the DO abort, which lands only after the teardown and listing-refresh
phases. No per-gate check can see the removed user then.

Hoist the check to the top of authorizeObservation's synchronous block (before
the prohibitAllSharing latch -- a blocked observation must not latch), so every
observation fails closed across the window; the per-gate check is subsumed and
removed. New test drives a plain observation (no exclusions) through the exact
post-removeObserver window.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…emantics.

The Step-5 failure bullet predated the fail()-scrub and the first-ever-only
rollback; rewrite it to describe the synchronous persisted-record scrub, the
re-prompt loop, the first-ever rollback vs. kept-registrations-on-reverify
split, and the torn-down-record exception. Edge case 5 now covers the
connection-added-mid-park case explicitly as part of the same residual.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>